Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update changelog.d format and Matrix link #10383

Merged
merged 3 commits into from
Sep 30, 2024

Conversation

geekosaur
Copy link
Collaborator

changelog-d was changed to make packages a list, so reflect this in the example. Also remove the "omit if it's an overarching change" part: we must list all packages if we want it to show up when generating release changelogs.

While I was there, I updated the Matrix channel reference because it was still reflecting the EMS Libera bridge.

Template B: This PR does not modify behaviour or interface

E.g. the PR only touches documentation or tests, does refactorings, etc.

Include the following checklist in your PR:

  • Patches conform to the coding conventions.
  • Is this a PR that fixes CI? If so, it will need to be backported to older cabal release branches (ask maintainers for directions).

@geekosaur geekosaur requested a review from fgaz September 26, 2024 23:19
Copy link
Collaborator

@ulysses4ever ulysses4ever left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's sad that changelog-d chose to use square brackets. Just a comma-separated list would be the least surprise.

@geekosaur
Copy link
Collaborator Author

geekosaur commented Sep 27, 2024

It'd still have been a syntax change: originally it was space-separated. @fgaz changed the type for some reason when he rewrote https://codeberg.org/fgaz/changelog-d/pulls/13.

@fgaz
Copy link
Member

fgaz commented Sep 27, 2024

changelog-d was changed to make packages a [bracketed] list

changelog-d chose to use square brackets

...I don't think this ever happened?! In https://codeberg.org/fgaz/changelog-d/pulls/13 I never touched Entry nor parseEntryFile.

cd "$(mktemp -d)"

cat > entry1 <<EOF
packages: foo
synopsis: entry 1
EOF

cat > entry2 <<EOF
packages: [foo]
synopsis: entry 2
EOF

cat > config <<EOF
organization: org
repository: repo
EOF
$ changelog-d-v1.0.1-x86_64-linux . --package foo
Errors encountered when parsing file ./entry2:

entry2:1:11: error:
unexpected '['
expecting white space or end of input

    1 | packages: [foo]
      |           ^
$ rm entry2
$ changelog-d-v1.0.1-x86_64-linux . --package foo
- entry 1

@geekosaur
Copy link
Collaborator Author

@fgaz
Copy link
Member

fgaz commented Sep 27, 2024

I see what happened. That CI job ran against 49d2ab0, which includes a changelog entry in markdown format. The markdown format uses a YAML frontmatter, and YAML lists do require brackets when using the inline syntax. This was introduced in changelog-d#5 and does not affect cabal-formatted entries.

The markdown example in the changelog-d readme was slightly incorrect, I just fixed it.

In the future I may allow space-separated packages as it's done for issues and prs, though this is a breaking change.

@geekosaur
Copy link
Collaborator Author

Documentation corrected and a note about Markdown format added.

CONTRIBUTING.md Outdated Show resolved Hide resolved
`changelog-d` was changed to make `packages` a list, so reflect
this in the example. Also remove the "omit if it's an overarching
change" part: we must list all packages if we want it to show up
when generating release changelogs.

While I was there, I updated the Matrix channel reference because
it was still reflecting the EMS Libera bridge.
Copy link
Collaborator

@ulysses4ever ulysses4ever left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think comma-separated should be explicitly mentioned.

CONTRIBUTING.md Outdated Show resolved Hide resolved
Co-authored-by: Artem Pelenitsyn <[email protected]>
@geekosaur geekosaur added squash+merge me Tell Mergify Bot to squash-merge and removed attention: needs-review labels Sep 28, 2024
@mergify mergify bot added ready and waiting Mergify is waiting out the cooldown period merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days labels Sep 28, 2024
@mergify mergify bot merged commit d215dc3 into haskell:master Sep 30, 2024
14 checks passed
synopsis: Add feature xyz
packages: [cabal-install]
prs: #0000
issues: #0000 #0000
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, this needs a followup: in YAML, # is a comment marker, so the changelog-d YAML format doesn't use the # in issue/PR numbers.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copied that from changelog-d's README; I guess that was what this was talking about.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days ready and waiting Mergify is waiting out the cooldown period squash+merge me Tell Mergify Bot to squash-merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants